fix: fix fiat values in testnet visibility #26273

OGPoyraz · 2024-08-01T10:32:46Z

Description

This PR goal is to fix visibility issue of viat values in testnet. For more info please visit task here #24945

Related issues

Fixes:

Manual testing steps

Turn off "Show conversion on test networks" setting in Advanced Settings
Use L2 test network (Arbitrum Sepolia could be added from here https://chainlist.org/chain/421614)
Go to test dapp
Click on "Create a token"
Observe fiat value is hidden (it was shown before)

Screenshots/Recordings

Before

After

Pre-merge author checklist

I've followed MetaMask Contributor Docs and MetaMask Extension Coding Standards.
I've completed the PR template to the best of my ability
I’ve included tests if applicable
I’ve documented my code using JSDoc format if applicable
I’ve applied the right labels on the PR (see labeling guidelines). Not required for external contributors.

Pre-merge reviewer checklist

I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed).
I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.

sonarcloud · 2024-08-01T10:41:56Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

ui/pages/confirmations/components/confirm-subtitle/confirm-subtitle.js

ui/pages/confirmations/components/confirm-subtitle/confirm-subtitle.test.js

ui/pages/confirmations/components/fee-details-component/fee-details-component.js

ui/selectors/selectors.js

…spite-show-conversion-on-test-networks-advanced-settings-being-disabled

OGPoyraz · 2024-09-03T08:57:52Z

ui/pages/confirmations/components/fee-details-component/fee-details-component.js

@@ -168,18 +173,15 @@ export default function FeeDetailsComponent({
                  {t('layer1Fees')}
                </Text>
              }
-              detailText={
-                useCurrencyRateCheck && renderTotalDetailText(layer1GasFee)


useCurrencyRateCheck is already checked in getShouldShowFiat selector hence am removing this.

…spite-show-conversion-on-test-networks-advanced-settings-being-disabled

OGPoyraz · 2024-09-03T13:24:49Z

test/e2e/tests/settings/account-token-list.spec.js

-          tag: 'div',
-        });
-        await driver.clickElement({ text: 'Fiat', tag: 'label' });
-        // We now need to enable "Show fiat on testnet" if we are using testnets (and since our custom


This is incorrect, 0x539 (the network used for e2e tests) is not testnet and not in the testnet list (See here https://github.com/MetaMask/metamask-extension/blob/24945-bug-fiat-values-continue-to-show-despite-show-conversion-on-test-networks-advanced-settings-being-disabled/shared/constants/network.ts#L1033)

0x539 is also not custom network (See getIsCustomNetwork function here https://github.com/MetaMask/metamask-extension/blob/24945-bug-fiat-values-continue-to-show-despite-show-conversion-on-test-networks-advanced-settings-being-disabled/ui/selectors/selectors.js#L2372)

Since this PR's aim is to correct previous getShouldShowFiat selector, this means that this e2e test must toggle useCurrencyRateCheck to make it work, and it's done via withConversionRate Enabled/Disabled fixture.

codecov · 2024-09-03T13:48:08Z

Codecov Report

Attention: Patch coverage is 95.00000% with 1 line in your changes missing coverage. Please review.

Project coverage is 69.95%. Comparing base (ca6fd53) to head (b3f44ac).
Report is 3 commits behind head on develop.

Files with missing lines	Patch %	Lines
...nts/fee-details-component/fee-details-component.js	85.71%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop   #26273      +/-   ##
===========================================
- Coverage    69.96%   69.95%   -0.00%     
===========================================
  Files         1442     1441       -1     
  Lines        50100    50098       -2     
  Branches     14006    14007       +1     
===========================================
- Hits         35049    35046       -3     
- Misses       15051    15052       +1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

metamaskbot · 2024-09-03T13:59:49Z

Builds ready [114e26f]

builds: chrome, firefox
builds (beta): chrome
builds (flask): chrome, firefox
builds (MMI): chrome, firefox
builds (test): chrome, firefox
builds (test-flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4, 5
- common: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9
- content-script: 0
- offscreen: 0
- ui: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1834 ± 100 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	215	2448	1658	508	244
		domContentLoaded	1618	2378	1814	200	96
		load	1629	2450	1834	208	100
		domInteractive	12	79	41	18	9

Bundle size diffs [🚨 Warning! Bundle size has increased!]

background: 0 Bytes (0.00%)
ui: -686 Bytes (-0.01%)
common: 118 Bytes (0.00%)

…spite-show-conversion-on-test-networks-advanced-settings-being-disabled

github-actions · 2024-09-06T01:18:55Z

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

…spite-show-conversion-on-test-networks-advanced-settings-being-disabled

sonarcloud · 2024-09-16T11:51:44Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

metamaskbot · 2024-09-16T12:34:41Z

Builds ready [b3f44ac]

builds: chrome, firefox
builds (beta): chrome
builds (flask): chrome, firefox
builds (MMI): chrome, firefox
builds (test): chrome, firefox
builds (test-flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4, 5
- common: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9
- content-script: 0
- offscreen: 0
- ui: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1697 ± 69 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	243	2046	1618	344	165
		domContentLoaded	1458	1989	1679	148	71
		load	1467	1994	1697	144	69
		domInteractive	21	96	34	19	9

Bundle size diffs [🚨 Warning! Bundle size has increased!]

background: 0 Bytes (0.00%)
ui: -686 Bytes (-0.01%)
common: 64 Bytes (0.00%)

Fix fiat values in testnet visibility

ad576c0

OGPoyraz requested review from a team as code owners August 1, 2024 10:32

OGPoyraz linked an issue Aug 1, 2024 that may be closed by this pull request

[Bug]: Fiat values continue to show despite "Show conversion on test networks" (Advanced Settings) being disabled #24945

Closed

metamaskbot added the team-confirmations Push issues to confirmations team label Aug 1, 2024

Fix lint

f75cce6

matthewwalsh0 requested changes Aug 2, 2024

View reviewed changes

OGPoyraz added 2 commits September 3, 2024 10:24

Merge branch 'develop' into 24945-bug-fiat-values-continue-to-show-de…

6bb9ace

…spite-show-conversion-on-test-networks-advanced-settings-being-disabled

Fix suggestions

3c74845

OGPoyraz commented Sep 3, 2024

View reviewed changes

OGPoyraz added 10 commits September 3, 2024 10:59

remove unnecessary useCurrencyRateCheck

3042a8f

Remove comment

76c1beb

Add localhost to testnets

4665294

revert

775f822

Merge branch 'develop' into 24945-bug-fiat-values-continue-to-show-de…

2ddf53b

…spite-show-conversion-on-test-networks-advanced-settings-being-disabled

Fix e2e

81854ef

Fix unit tests

bc2cce1

Fix tests

7eb149b

Update snapshot

bf2e3d7

Fix e2e tests

c4e530a

OGPoyraz commented Sep 3, 2024

View reviewed changes

Remove localization e2e test

114e26f

OGPoyraz marked this pull request as draft September 3, 2024 13:33

OGPoyraz added 3 commits September 4, 2024 08:56

Fix e2e tests

e2713a5

Remove delay

4b274ec

Fix unit tests

35543ad

OGPoyraz marked this pull request as ready for review September 4, 2024 07:27

pedronfigueiredo previously approved these changes Sep 4, 2024

View reviewed changes

OGPoyraz and others added 2 commits September 5, 2024 10:11

Merge branch 'develop' into 24945-bug-fiat-values-continue-to-show-de…

81dcfaf

…spite-show-conversion-on-test-networks-advanced-settings-being-disabled

Merge branch 'develop' into 24945-bug-fiat-values-continue-to-show-de…

5e49bc8

…spite-show-conversion-on-test-networks-advanced-settings-being-disabled

legobeat requested a review from matthewwalsh0 September 6, 2024 01:19

matthewwalsh0 previously approved these changes Sep 6, 2024

View reviewed changes

OGPoyraz added 8 commits September 6, 2024 15:45

Merge branch 'develop' into 24945-bug-fiat-values-continue-to-show-de…

abab1f5

…spite-show-conversion-on-test-networks-advanced-settings-being-disabled

Merge branch 'develop' into 24945-bug-fiat-values-continue-to-show-de…

6aa5d5b

…spite-show-conversion-on-test-networks-advanced-settings-being-disabled

Merge branch 'develop' into 24945-bug-fiat-values-continue-to-show-de…

a22e2e0

…spite-show-conversion-on-test-networks-advanced-settings-being-disabled

Merge branch 'develop' into 24945-bug-fiat-values-continue-to-show-de…

e72d35e

…spite-show-conversion-on-test-networks-advanced-settings-being-disabled

Merge branch 'develop' into 24945-bug-fiat-values-continue-to-show-de…

0fbc9df

…spite-show-conversion-on-test-networks-advanced-settings-being-disabled

Merge branch 'develop' into 24945-bug-fiat-values-continue-to-show-de…

9c67f8c

…spite-show-conversion-on-test-networks-advanced-settings-being-disabled

Merge branch 'develop' into 24945-bug-fiat-values-continue-to-show-de…

9e18129

…spite-show-conversion-on-test-networks-advanced-settings-being-disabled

Fix e2e test

bad0757

OGPoyraz dismissed stale reviews from matthewwalsh0 and pedronfigueiredo via bad0757 September 11, 2024 13:09

pedronfigueiredo previously approved these changes Sep 11, 2024

View reviewed changes

Fix mmi e2e test

625217c

OGPoyraz dismissed pedronfigueiredo’s stale review via 625217c September 16, 2024 11:45

Merge branch 'develop' into 24945-bug-fiat-values-continue-to-show-de…

b3f44ac

…spite-show-conversion-on-test-networks-advanced-settings-being-disabled

matthewwalsh0 approved these changes Sep 16, 2024

View reviewed changes

pedronfigueiredo approved these changes Sep 16, 2024

View reviewed changes

OGPoyraz merged commit e048a93 into develop Sep 16, 2024
78 checks passed

OGPoyraz deleted the 24945-bug-fiat-values-continue-to-show-despite-show-conversion-on-test-networks-advanced-settings-being-disabled branch September 16, 2024 12:52

github-actions bot locked and limited conversation to collaborators Sep 16, 2024

metamaskbot added the release-12.6.0 Issue or pull request that will be included in release 12.6.0 label Sep 16, 2024

metamaskbot added release-12.5.0 Issue or pull request that will be included in release 12.5.0 and removed release-12.6.0 Issue or pull request that will be included in release 12.6.0 labels Sep 29, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: fix fiat values in testnet visibility #26273

fix: fix fiat values in testnet visibility #26273

OGPoyraz commented Aug 1, 2024 •

edited by github-actions bot

Loading

sonarcloud bot commented Aug 1, 2024

OGPoyraz Sep 3, 2024

OGPoyraz Sep 3, 2024 •

edited

Loading

codecov bot commented Sep 3, 2024 •

edited

Loading

metamaskbot commented Sep 3, 2024

github-actions bot commented Sep 6, 2024

sonarcloud bot commented Sep 16, 2024

metamaskbot commented Sep 16, 2024

fix: fix fiat values in testnet visibility #26273

fix: fix fiat values in testnet visibility #26273

Conversation

OGPoyraz commented Aug 1, 2024 • edited by github-actions bot Loading

Description

Related issues

Manual testing steps

Screenshots/Recordings

Before

After

Pre-merge author checklist

Pre-merge reviewer checklist

sonarcloud bot commented Aug 1, 2024

Quality Gate passed

OGPoyraz Sep 3, 2024

Choose a reason for hiding this comment

OGPoyraz Sep 3, 2024 • edited Loading

Choose a reason for hiding this comment

codecov bot commented Sep 3, 2024 • edited Loading

Codecov Report

metamaskbot commented Sep 3, 2024

github-actions bot commented Sep 6, 2024

sonarcloud bot commented Sep 16, 2024

Quality Gate passed

metamaskbot commented Sep 16, 2024

OGPoyraz commented Aug 1, 2024 •

edited by github-actions bot

Loading

OGPoyraz Sep 3, 2024 •

edited

Loading

codecov bot commented Sep 3, 2024 •

edited

Loading